Skip to content

Comments

feat: support cycles amount suffixes in .yaml files#375

Merged
raymondk merged 9 commits intomainfrom
SDK-2573-use-the-cycles-parser-for-all-the-cycles-icp-yaml-inputs
Feb 21, 2026
Merged

feat: support cycles amount suffixes in .yaml files#375
raymondk merged 9 commits intomainfrom
SDK-2573-use-the-cycles-parser-for-all-the-cycles-icp-yaml-inputs

Conversation

@viviveevee
Copy link
Contributor

@viviveevee viviveevee commented Feb 19, 2026

Support cycles amount suffixes in .yaml files

@viviveevee viviveevee marked this pull request as ready for review February 19, 2026 16:30
@viviveevee viviveevee requested a review from a team as a code owner February 19, 2026 16:30
{
#[derive(Deserialize)]
#[serde(untagged)]
enum OneOf {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making this internal means that the schema can't see it. Can this function instead be formatted as a pub wrapper type which impls Deserialize and JsonSchema?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's now a pub wrapper type. How do you like the new version? Still gives a clunky impression to me, but I'd like to validate the amount the first time the type is constructed. Is there a nicer way to do it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good as is to me, but if you did want to make it cleaner, you could implement JsonSchema directly on a u64 newtype, since the enum structure is I think only there for schemars(untagged).

@raymondk raymondk enabled auto-merge (squash) February 21, 2026 03:36
@raymondk raymondk merged commit b9d0564 into main Feb 21, 2026
84 checks passed
@raymondk raymondk deleted the SDK-2573-use-the-cycles-parser-for-all-the-cycles-icp-yaml-inputs branch February 21, 2026 03:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants